-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Teads Bid Adapter: support floc and uid2 user IDs #7116
Teads Bid Adapter: support floc and uid2 user IDs #7116
Conversation
modules/teadsBidAdapter.js
Outdated
const BIDDER_CODE = 'teads'; | ||
const GVL_ID = 132; | ||
const ENDPOINT_URL = 'https://a.teads.tv/hb/bid-request'; | ||
const ENDPOINT_URL = '//SSP_PORT_8080_TCP_ADDR:SSP_PORT_8080_TCP_PORT/hb/bid-request'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly is going on here?
This looks like some macro to be replaced but I do not see it happening anywhere in your adapter?
SSP_PORT_8080_TCP_ADDR:SSP_PORT_8080_TCP_PORT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it's an error on our side, we will revert that change! (edit: fixed in 2nd commit)
modules/teadsBidAdapter.js
Outdated
* @returns `{} | {firstPartyCookieTeadsId: string}` | ||
*/ | ||
function getFirstPartyTeadsIdParameter() { | ||
const storage = getStorageManager(GVL_ID, BIDDER_CODE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think it would be better to just init your storage manager top level of module
You are calling this function every single time anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple questions
fe37af5
to
247dfc8
Compare
Hello @robertrmartinez , we have done the changes following your questions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adapter looks good but would like the storage code to be stubbed in the tests.
We are trying to be more diligent about not letting people modify the window during tests, setting local storage and cookies is not reliable, especially when you have many adapters doing so.
Please see the examples I just gave and update the spec file.
Thanks, let me know if you have any questions
}); | ||
|
||
it('should add firstPartyCookieTeadsId param to payload if first-party cookie is available', function () { | ||
storage.setCookie('_tfpvi', 'my-teads-id'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not want to actually be setting and reading from storage / cookies inside of tests.
We should be mocking the stubbing / mocking the storage manager, which has its own tests to verify it works.
Here is an example:
You'll need to export your storage var so that your tests can mock it:
https://github.com/prebid/Prebid.js/blob/master/modules/rubiconAnalyticsAdapter.js#L11
Then in your beforeEach you should setup the stub
https://github.com/prebid/Prebid.js/blob/master/test/spec/modules/rubiconAnalyticsAdapter_spec.js#L596-L600
Don't forget to restore it back in the afterEach
https://github.com/prebid/Prebid.js/blob/master/test/spec/modules/rubiconAnalyticsAdapter_spec.js#L629-L631
Then in your tests you can set them up to return what you want and make sure your module works as expected:
https://github.com/prebid/Prebid.js/blob/master/test/spec/modules/rubiconAnalyticsAdapter_spec.js#L1297
// Following the introduction of tests involving reading/writing cookies, | ||
// this allows for running this spec as a single file with: | ||
// `gulp test --file "test/spec/modules/teadsBidAdapter_spec.js"`. | ||
window.$$PREBID_GLOBAL$$.processQueue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the stubbing of storage, you can get rid of this I believe.
This type of code in test files can cause downstream tests to be very flakey.
We do not want to modify the window object and its contents if we do not have to in tests.
Thanks @robertrmartinez, it's so much better/stable to use a stub rather than the global environment! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome stuff
Thanks for being so flexible!
* Teads adapter: support some user IDs * review changes * Stub access to storage functions in tests of Teads adapter Co-authored-by: Kylian Deau <kylian.deau@teads.tv>
Type of change
Description of change
Support FLoC ID (
flocId
) and Unified ID v2 (uid2Id
) user IDs in the Teads adapter.Additionally, read the
_tfpvi
first-party cookie when available on the website's domain.Documentation has been updated on the
prebid.github.io
repository: prebid/prebid.github.io#3072.Contact email of the adapter’s maintainer : innov-ssp@teads.tv.